Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jazzma/pwc menus #202

Closed
wants to merge 19 commits into from

Conversation

yashjagtap23
Copy link
Contributor

added menus for jazzman's and pwc

added pwc menu, will add milkshake place later as well. Added dropdown to select between menus. changed colors to match what theme is chosen
Added Special Menu (Poke Bowl, Sushi, Taco, Wings) and Deli menu (Sandwich Bar)
@aw-0
Copy link
Member

aw-0 commented Aug 16, 2023

Thanks so much for continuing to work on this new feature! Pretty awesome how you've gathered all of this info and presented it nicely. Will be helpful to a lot of students. Here's some feedback:

  • The dropdown on the top of the page could be implemented better. I'd suggest using one of the existing dropdown components (components/Dropdown.vue) to add this. Additionally, the "Select a Menu" text could fit slightly better as well.
  • Great job with the small details and adding the home button, but I feel as if the page could fit in a little better overall with the rest of the site. I'd definitely take some inspiration from the Jukebox/Schedules page in their green/white style with the content in a "card" like overlay

Super excited to see this feature grow! Thanks again for setting aside your time :)

Copy link
Member

@aw-0 aw-0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see my comment :)

@aw-0 aw-0 linked an issue Aug 16, 2023 that may be closed by this pull request
@aw-0
Copy link
Member

aw-0 commented Aug 20, 2023

Thanks again!

  • It'd be great to see more margin between the center "card" and the top of the webpage, similar to the Jukebox.
  • The dropdown menu isn't centered (see attached pic). I'd play around with its location more - potentially inline & to the left of the menu name. Not sure what may be the best location for it. Also -- I'd capitalize PWC here as well
CleanShot 2023-08-20 at 12 35 52@2x - There seems to be a black border around the center "card" that doesn't fit in very well - Lastly, the page needs to be mobile-compatible as part of the menu content comes off of the screen, other wise the mobile experience wouldn't be great CleanShot 2023-08-20 at 12 37 36@2x

@yashjagtap23
Copy link
Contributor Author

fixed mobile views
Screenshot 2023-08-21 at 9 28 47 PM
Screenshot 2023-08-21 at 9 29 10 PM

@yashjagtap23 yashjagtap23 requested a review from aw-0 August 22, 2023 23:07
@aw-0
Copy link
Member

aw-0 commented Aug 23, 2023

Thanks for the new additions!

  • Like earlier, would love to see if the position of the choice dropdown could be moved:

I'd play around with its location more - potentially inline & to the left of the menu name. Not sure what may be the best location for it.

  • The padding on the inline card isn't consistent with the rest of the site (once again, like Jukebox). That causes the text to be extra squished and harder to read. Take a look at this image and compare it with the menus - looks a bit off
CleanShot 2023-08-22 at 19 38 11@2x
  • Continuing with keeping things consistent, let's make sure the home button has consistent padding with the other pages. Kind of a nit-pick but users' muscle memory will cause them not to hit the button if it's not aligned correctly.
CleanShot 2023-08-22 at 19 39 23@2x
  • Whenever I head back to the homepage from the menu page, all of the Y padding of the card titles increases by a lot. Not really sure what's happening here
CleanShot 2023-08-22 at 19 40 36@2x

Copy link
Member

@aw-0 aw-0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see my previous comment

@yashjagtap23
Copy link
Contributor Author

wasn't able to change dropdown alignment since the component only allows for 3 alignments (left, right, center) and only one way to roll out - down or up)

@aw-0
Copy link
Member

aw-0 commented Aug 23, 2023

With the dropdown - I was talking about the overall alignment of the actual element itself, as it's a bit jarring just having that as the first item you see on the page. It's meant to be more of a smaller, form element than a giant selector.

@aw-0
Copy link
Member

aw-0 commented Aug 27, 2023

Thanks for fixing the position! However - I'm not sure if a dropdown is the best way to choose the menus, or if the current impl can be revised to make it more user friendly (cc @JosephShepin). Also, the menu is still quite hard to read on mobile - showed this to a few friends and they agreed. May be a good idea to continue playing around with text sizes & orientation on mobile as well.

@aw-0
Copy link
Member

aw-0 commented Aug 29, 2023

I think I found the best way to make the dropdown fit in better. Let's make the default text of the dropdown be the text next to the dropdown, and have it fit the entirety of the card on mobile. Not entirely sure how that'll look on desktop but it's a step in the right direction image

Also - in the screenshot there's some positioning issues with the deli menu. Thanks for toning down the text size that looks better

Added text to dropdown to match current menu
@aw-0
Copy link
Member

aw-0 commented Sep 13, 2023

Thanks for these changes!

Looks like there's some padding issues on the PWC menu and Deli. It also looks like the only menu that's padded correctly on mobile browsers (aka aligned to the middle) is the Special Menu.
CleanShot 2023-09-12 at 22 08 16@2x
CleanShot 2023-09-12 at 22 11 07@2x

Is there any way to get the dropdown centered on mobile as well? That may look better
CleanShot 2023-09-12 at 22 11 46@2x

Let's also have all of the menu headers match the names of the dropdown as well.

Fixed styling to work on all devices properly and no extra padding or white space
@yashjagtap23
Copy link
Contributor Author

also matched names to dropdown

@aw-0
Copy link
Member

aw-0 commented Sep 20, 2023

Thanks for those changes! We're getting close here 🤞

The responsiveness is still a bit off -- this is crucial since so many students use StevensonSpace on so many different devices.
E.x.
CleanShot 2023-09-20 at 15 24 33@2x
CleanShot 2023-09-20 at 15 25 16@2x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

idea: Jazzman's/Grab Lunch Items
3 participants